Note about Dispose affecting Attachments #2902
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://referencesource.microsoft.com/#system/net/System/Net/mail/MailMessage.cs,a22b7e48cd4bc831 Disposing of Attachments also closes the Streams they're using. The problem I had that caused me to dig into the code wasn't relevant to this change, but I saw it as a potential occasional issue. A MailMessage intuitively shouldn't take this kind of ownership of an Attachment that was given to it by an external source since the documentation suggests that Attachments can be reused. In my mind, "not intuitive" means "needs to be clearly documented."
|
@davidsh, could you take a look at this PR, please? |
This doc change is accurately describing the current behavior of .NET. |
davidsh
approved these changes
Aug 2, 2019
rpetrusha
added
the
✨ 1st-time dotnet-api-docs contributor!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Disposing of Attachments also closes the Streams they're using.
https://referencesource.microsoft.com/#system/net/System/Net/mail/MailMessage.cs,a22b7e48cd4bc831
The problem I had that caused me to dig into the code wasn't relevant to this change, but I saw it as a potential occasional issue. A MailMessage intuitively shouldn't take this kind of ownership of an Attachment that was given to it by an external source since the documentation suggests that Attachments can be reused. In my mind, "not intuitive" means "needs to be clearly documented."
(For the sake of discussion: the issue I had that made me dig into the code, while not directly related, was that reusing my MemoryStream for a second Attachment in a second MailMessage resulted in an email being sent with a 0-byte attachment. I don't know why MimePart.ResetStream() didn't reset the stream as I expected, but I'd say it was because the code spun off a thread to send each email, clearly having the potential for concurrency problems. I've since put my SmtpClient.Send call in a critical section, but knowing exactly what needed to be in a critical section also required digging into the code to see where exactly the stream was being used, and that should perhaps be documented somehow.)